-
-
Notifications
You must be signed in to change notification settings - Fork 655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for TypedArray interop between WASM and JS #268
Conversation
view; | ||
|
||
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Typed_arrays | ||
if (typedArrayCtor === Int8Array || typedArrayCtor === Uint8Array || typedArrayCtor === Uint8ClampedArray) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use length * typedArrayCtor.BYTES_PER_ELEMENT. If BYTES_PER_ELEMENT
doesn't exist in typedArrayCtor
throw "Unsupported TypedArray constructor"
Just leaving my draft here, that I made before this PR, in case it's useful :) /** Allocates a new fixed-size buffer in the module's memory and returns its pointer. */
function newBuffer(view, length) {
return view.constructor === Function
? newEmptyBuffer(view, length)
: newCopiedBuffer(view, length);
}
/** Allocates a new fixed-size empty buffer of the specified type and returns its pointer. */
function newEmptyBuffer(ctor, length, unsafe) {
var size = ctor.BYTES_PER_ELEMENT * length;
var ptr = exports["memory.allocate"](size);
if (!unsafe) exports["memory.fill"](ptr, 0, size);
return ptr;
}
/** Copies an existing fixed-size buffer to the module's memory and returns its pointer. */
function newCopiedBuffer(view, length) {
if (length === undefined) length = view.length;
var ctor = view.constructor;
var ptr = exports["memory.allocate"](ctor.BYTES_PER_ELEMENT * length);
new ctor(mem, ptr, length).set(view);
return ptr;
} |
function newBuffer(view, length, unsafe) {
return view.constructor === Function
? newEmptyBuffer(view, length, unsafe)
: newCopiedBuffer(view, length);
} ? |
That's just so you can do newBuffer(new Float32Array([1.1, 1,2, ...]); or newBuffer(Float32Array, 10); |
I see. I just add |
And when we need read buffer after procces in js side we just create array from linear buffer: const ptr = newBuffer(Float32Array, 10);
instance.exports.process(ptr, 10);
const bufF32 = new Float32Array(instance.exports.memory.buffer, ptr, 10);
// read processed result from bufF32 Right? |
Either that or return the respective view as well as proposed in the PR. |
function getBuffer(ctor, ptr, length) {
checkMem();
return new ctor(mem, ptr, length);
}
getBuffer(Float32Array, ptr, 10); Or better: function newBuffer(view, length, unsafe) {
const ptr = view.constructor === Function
? newEmptyBuffer(view, length, unsafe)
: newCopiedBuffer(view, length);
return {
ptr,
length,
array() {
checkMem();
return new view.constructor(mem, ptr, length);
},
free() {
exports["memory.free"](ptr);
}
};
} const buffer = newBuffer(Float32Array, 10);
instance.exports.someMutableProcess(buffer.ptr, buffer.length);
const result = buffer.array();
...
buffer.free(); |
I prefer create Typed Array lazily and only when we really need read back from memory |
And will be great ability create ArrayBuffer from Pointer class on AssemblyScript but this require extra space (for 4-byte length field) during alloc inside newArray. |
For reference, for a real typed array on the AS side we'd need to do something like this (didn't test, no GC support): function computeBufferSize(byteLength) {
const HEADER_SIZE = 8;
return 1 << (32 - Math.clz32(byteLength + HEADER_SIZE - 1));
}
/** Creates a new typed array in the module's memory and returns its pointer. */
function newArray(view, length, unsafe) {
var ctor = view.constructor;
if (ctor === Function) { // TypedArray constructor
ctor = view;
view = null;
} else { // TypedArray instance
if (length === undefined) length = view.length;
}
var elementSize = ctor.BYTES_PER_ELEMENT;
if (!elementSize) throw Error("not a typed array");
var byteLength = elementSize * length;
var ptr = exports["memory.allocate"](12); // TypedArray header
var buf = exports["memory.allocate"](computeBufferSize(byteLength)); // ArrayBuffer
checkMem();
U32[ ptr >>> 2] = buf; // .buffer
U32[(ptr + 4) >>> 2] = 0; // .byteOffset
U32[(ptr + 8) >>> 2] = byteLength; // .byteLength
U32[ buf >>> 2] = byteLength; // .byteLength
U32[(buf + 4) >>> 2] = 0; // 0
if (view) {
new ctor(mem, buf + 8, length).set(view);
} else if (!unsafe) {
exports["memory.fill"](buf + 8, 0, byteLength);
}
return ptr;
}
/** Gets a view on a typed array in the module's memory by its pointer. */
function getArray(ctor, ptr) {
var elementSize = ctor.BYTES_PER_ELEMENT;
if (!elementSize) throw Error("not a typed array");
checkMem();
var buf = U32[ ptr >>> 2];
var byteOffset = U32[(ptr + 4) >>> 2];
var byteLength = U32[(ptr + 8) >>> 2];
return new ctor(mem, buf + 8 + byteOffset, (byteLength - byteOffset) / elementSize);
} |
Oh wow, thanks for the lively discussion. Your suggestions look great. I would suggest that I combine them and update the PR. To wrap it up: Minimal lib/loader extension But I question myself, why do we call it So from the JS side of view, I would be more happy with Minimal AS side datatype extensions Powerful and easy API For the developer, the experience would be awesome as this will be possible and has only the overhead decided for: JS:
AS:
What do you think? I believe it contains all the ideas? |
...
U32[(ptr + 8) >>> 2] = byteLength; // .byteLength
U32[ buf >>> 2] = byteLength; // .byteLength
... Isn't this will be: U32[(ptr + 8) >>> 2] = length; // .length
U32[ buf >>> 2] = byteLength; // .byteLength ? |
Layout of a typed array differs a bit from a normal array. A typed array computes the length property from its byteLength and type. |
Got it! I thought this usual Array |
Hmm, btw @inline
get length(): i32 {
return this.buffer_.byteLength >> alignof<T>();
} EDIT @inline
get length(): i32 {
return (this.buffer.byteLength - this.byteOffset) >> alignof<T>();
} |
We can't drop it there because a user can modify |
Got it, so it actually works as capacity |
As this seems to lead to some confusion, I decided to go ahead with an implementation that creates a proper typed array for use on the AS side here. So, this should "just work": // JS side
var ptr = module.newArray(new Int32Array([1, 2, 3]));
module.processArray(ptr);
console.log(module.getArray(ptr));
module.freeArray(ptr); // once not used anymore
// AS side
export function processArray(arr: Int32Array): void {
...
} |
Okay. Edit: I was to fast in my response. It works, I just didn't see the assignment wasn't on the array index location. Maybe the AS code could do:
Also, as I mentioned earlier, I find it counter-intuitive that Also, |
Hmm, might be, yeah. My thought was that all we can do currently is typed arrays anways, but if you think it helps we can rename it to
Yeah, but it needs to know the type somehow, because without it doesn't know how to interpret the otherwise untyped buffer. In Wasm memory, instances of classes don't have runtime information that we could use dynamically. |
I know that naming things is always a great problem, so just a semantic suggestion : wouldn't be more pertinent to call it |
Did you try my initial PR? It returns the reference to the constructor function of the TypedArray together with it's instance and pointer so you don't need to put it again on JS side.
Well, it is a TypedArray used in an unconventional way (to "share" memory with AS) -- but on JS side, it is indeed a standard TypedArray :) That's why I opted for newTypedArray(...) :) |
Yeah, I was just concerned about the case where a typed array is returned from AS. That'd be just a number and it seemed more consistent this way. |
Oh well, on AS side my PR only differs in passing two parameters because I didn't knew I could just use the TypedArray counterpart as an argument in AS. I also like your proposal much more for the AS side. This is clearly more elegant. I think the proposed API of my PR (JS side) and yours (JS and AS side) could go hand in hand... Bringing @jbousquie remark into play... as it is a new use case... like "sharing a TypedArray" between JS and AS we could maybe name the methods for what they do context-wise...
It would become:
But it's up to you. It's working as it is now and from my point of view it's already doing it's job... |
This could be confused because JS have |
|
Regarding naming: I think Hence, closing for now, but feel free to discuss :) |
As discussed in #263 and #240, I added support for memory mapped interop of all standard TypedArray subtypes. I added tests for every subtype to ensure a safe operation. Type checks ensure the safe construction of TypedArray subtypes in the loader lib.